-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Hypervisor default
as cache mode for disk offerings
#10282
base: main
Are you sure you want to change the base?
Conversation
@blueorangutan package |
@hsato03 a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #10282 +/- ##
=========================================
Coverage 16.15% 16.15%
- Complexity 13263 13265 +2
=========================================
Files 5666 5666
Lines 497960 497965 +5
Branches 60241 60242 +1
=========================================
+ Hits 80459 80470 +11
+ Misses 408499 408486 -13
- Partials 9002 9009 +7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 12225 |
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
@hsato03 could you solve the conflicts? |
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java
Outdated
Show resolved
Hide resolved
enum DiskCacheMode { | ||
NONE("none"), WRITEBACK("writeback"), WRITETHROUGH("writethrough"); | ||
HYPERVISOR_DEFAULT("hypervisor_default"), NONE("none"), WRITEBACK("writeback"), WRITETHROUGH("writethrough"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we have this enum multiple times? Maybe we could extract it to a common place?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regarding the DiskOffering.DiskCacheMode
enum, it represents the value of the users input in an API call. On the other hand, the LibvirtVMDef.DiskCacheMode
enum represents the value that will be used in the VM XML. In this case, the value of the HYPERVISOR_DEFAULT
field is slightly different between these enums.
Concerning the ProviderAdapterDiskOffering.DiskCacheMode
, it could be refactored to use the DiskOffering.DiskCacheMode
enum, but I think this can be done in another PR since there are other enums that could be reused in this class.
@blueorangutan package |
@hsato03 a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 12472 |
Description
Currently, ACS provides
none
,write-back
andwrite-through
options as cache mode when creating disk offerings.This PR adds the option
Hypervisor default
as an alternative to cache mode. By using this option, the KVM hypervisor will choose the cache mode to be used by the disk.Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Screenshots (if appropriate):
Disk offering creation
Compute offering with compute only disk offering creation
How Has This Been Tested?
Hypervisor default
as cache mode;virt-manager
that the disk of both VMs was using theHypervisor default
cache mode, as in the screenshot below.